allow skipping must-gather collection for tests by dbasunag · Pull Request #644 · opendatahub-io/opendatahub-tests

dbasunag · 2025-09-25T18:34:36Z

Description

How Has This Been Tested?

Merge criteria:

The commits are squashed in a cohesive manner and have meaningful messages.
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work

Summary by CodeRabbit

New Features
- Added a test marker to selectively skip must-gather collection, enabling finer control during test runs.
Tests
- Applied the new marker to specific test suites to avoid unnecessary must-gather collection, improving test performance.
Documentation
- Documented the new pytest marker in the test configuration for discoverability and consistent usage.

coderabbitai · 2025-09-25T18:34:42Z

📝 Walkthrough

Walkthrough

Adds helper functions to detect pytest node markers and skip must-gather when marked. Updates pytest_exception_interact to honor the new marker alongside the CLI option. Declares the skip_must_gather marker in pytest.ini. Applies the marker to two model registry test modules.

Changes

Cohort / File(s)	Summary of changes
Pytest must-gather control `conftest.py`	Added `get_all_node_markers` and `is_skip_must_gather`; updated `pytest_exception_interact` to check `--collect-must-gather` and skip when node has `skip_must_gather`.
Pytest configuration `pytest.ini`	Declared new marker `skip_must_gather` under pytest markers.
Test annotations `tests/model_registry/image_validation/test_verify_rhoai_images.py`, `tests/model_registry/model_catalog/test_default_model_catalog.py`	Added `@pytest.mark.skip_must_gather` to a test method and a test class, respectively.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly summarizes the main change which is adding support to skip must-gather collection for marked tests, matching the implementation of the `skip_must_gather` marker and related logic updates.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2025-09-25T18:34:55Z

The following are automatically added/executed:

PR size label.
Run pre-commit
Run tox
Add PR author as the PR assignee
Build image based on the PR

Available user actions:

To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
lgtm label removed on each new commit push.
To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
verified label removed on each new commit push.
To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.

Supported labels

{'/build-push-pr-image', '/verified', '/cherry-pick', '/lgtm', '/hold', '/wip'}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

pytest.ini (1)
23-23: Nit: fix marker description grammar and align phrasing with others.

Update to read more clearly and match the “Mark tests that …” style used above.
-    skip_must_gather: Tests that does not require must-gather for triaging
+    skip_must_gather: Mark tests that do not require must-gather collection for triaging
conftest.py (2)
446-446: Silence Ruff ARG001: prefix unused call parameter with underscore.

Keeps the hook signature while fixing the linter warning. [Based on static_analysis_hints]
-def pytest_exception_interact(node: Item | Collector, call: CallInfo[Any], report: TestReport | CollectReport) -> None:
+def pytest_exception_interact(node: Item | Collector, _call: CallInfo[Any], report: TestReport | CollectReport) -> None:
438-444: Streamline marker check; drop unused helper
Replace both functions with:
def is_skip_must_gather(node: Node) -> bool:
    return any(node.iter_markers(name="skip_must_gather"))
get_all_node_markers isn’t referenced elsewhere and can be removed.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6265ddf and 51a1bff.

📒 Files selected for processing (4)

conftest.py (2 hunks)
pytest.ini (1 hunks)
tests/model_registry/image_validation/test_verify_rhoai_images.py (1 hunks)
tests/model_registry/model_catalog/test_default_model_catalog.py (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-21T19:08:14.982Z

Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#527
File: tests/model_registry/rbac/conftest.py:358-359
Timestamp: 2025-08-21T19:08:14.982Z
Learning: In tests/model_registry/rbac/conftest.py, the model catalog configmap only exists in one specific namespace (py_config["model_registry_namespace"]) regardless of where model registry instances are deployed, making it a shared resource that serves all model registry instances across namespaces.

Applied to files:

tests/model_registry/model_catalog/test_default_model_catalog.py

🧬 Code graph analysis (1)

conftest.py (1)

tests/conftest.py (1)

nodes (562-563)

🪛 Ruff (0.13.1)

conftest.py

446-446: Unused function argument: call

(ARG001)

🔇 Additional comments (4)

conftest.py (2)

22-22: LGTM: importing Node for precise type hints.

Accurate and scoped import.

448-448: LGTM: condition now honors the skip_must_gather marker.

The gate correctly prevents collection when marked.

tests/model_registry/model_catalog/test_default_model_catalog.py (1)

33-33: LGTM: applied skip_must_gather at class scope.

Consistent with the new marker behavior.

tests/model_registry/image_validation/test_verify_rhoai_images.py (1)

32-33: LGTM: test opts out of must-gather collection.

Matches the conftest logic and pytest.ini marker declaration.

kpunwatk

/lgtm

fege

/lgtm

github-actions · 2025-09-29T14:24:03Z

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

dbasunag · 2025-09-30T13:39:05Z

/cherry-pick 2.25

rhods-ci-bot · 2025-09-30T13:39:27Z

Cherry pick action created PR #661 successfully 🎉!
See: https://github.com/opendatahub-io/opendatahub-tests/actions/runs/18131870837

Co-authored-by: Debarati Basu-Nag <dbasunag@redhat.com> Co-authored-by: Jorge <jgarciao@users.noreply.github.com>

allow skipping must-gather collection for tests

51a1bff

dbasunag requested review from a team, fege and lugi0 as code owners September 25, 2025 18:34

github-actions Bot added the ModelRegistry label Sep 25, 2025

github-actions Bot assigned dbasunag Sep 25, 2025

github-actions Bot added the size/xs label Sep 25, 2025

coderabbitai Bot reviewed Sep 25, 2025

View reviewed changes

kpunwatk approved these changes Sep 26, 2025

View reviewed changes

rhods-ci-bot added the lgtm-by-kpunwatk label Sep 26, 2025

Merge branch 'main' into skip_mg

d904997

rhods-ci-bot removed the lgtm-by-kpunwatk label Sep 29, 2025

fege approved these changes Sep 29, 2025

View reviewed changes

rhods-ci-bot added the lgtm-by-fege label Sep 29, 2025

dbasunag merged commit 5d89d3e into opendatahub-io:main Sep 29, 2025
9 checks passed

dbasunag deleted the skip_mg branch September 29, 2025 14:22

rhods-ci-bot pushed a commit that referenced this pull request Sep 30, 2025

allow skipping must-gather collection for tests (#644)

d1acc5b

rhods-ci-bot added the cherry-pick label Sep 30, 2025

rhods-ci-bot mentioned this pull request Sep 30, 2025

[Cherry-pick][2.25] allow skipping must-gather collection for tests #661

Merged

rhods-ci-bot added the commented-by-rhods-ci-bot label Sep 30, 2025

dbasunag added a commit that referenced this pull request Oct 1, 2025

allow skipping must-gather collection for tests (#644) (#661)

bfdee7e

Co-authored-by: Debarati Basu-Nag <dbasunag@redhat.com> Co-authored-by: Jorge <jgarciao@users.noreply.github.com>

sheltoncyril pushed a commit to sheltoncyril/opendatahub-tests that referenced this pull request Oct 2, 2025

allow skipping must-gather collection for tests (opendatahub-io#644)

c702d46

mwaykole pushed a commit to mwaykole/opendatahub-tests that referenced this pull request Jan 23, 2026

allow skipping must-gather collection for tests (opendatahub-io#644)

52c9d15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow skipping must-gather collection for tests#644

allow skipping must-gather collection for tests#644
dbasunag merged 2 commits intoopendatahub-io:mainfrom
dbasunag:skip_mg

dbasunag commented Sep 25, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 25, 2025 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Uh oh!

github-actions Bot commented Sep 25, 2025

Uh oh!

coderabbitai Bot left a comment

Uh oh!

kpunwatk left a comment

Uh oh!

fege left a comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 29, 2025

Uh oh!

dbasunag commented Sep 30, 2025

Uh oh!

rhods-ci-bot commented Sep 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

dbasunag commented Sep 25, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Merge criteria:

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

github-actions Bot commented Sep 25, 2025

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

kpunwatk left a comment

Choose a reason for hiding this comment

Uh oh!

fege left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 29, 2025

Uh oh!

dbasunag commented Sep 30, 2025

Uh oh!

rhods-ci-bot commented Sep 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dbasunag commented Sep 25, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 25, 2025 •

edited

Loading